fs: cache context lookup in vectored io loops #61387
Conversation
nodejs-github-bot
added
c++
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #61387 +/- ##
==========================================
- Coverage 88.54% 88.51% -0.03%
==========================================
Files 704 704
Lines 208793 208797 +4
Branches 40307 40315 +8
==========================================
- Hits 184866 184810 -56
- Misses 15913 15966 +53
- Partials 8014 8021 +7
🚀 New features to boost your workflow:
|
addaleax
left a comment
addaleax
left a comment
There was a problem hiding this comment.
You can also move Realm::context() into node_realm-inl.h so that the compiler can already make this optimization itself.
thanks, I used |
addaleax
left a comment
addaleax
left a comment
There was a problem hiding this comment.
Be sure to update the commit message/PR title here as well
| return isolate_; | ||
| } | ||
|
|
||
| inline v8::Local<v8::Context> Realm::context() const { |
There was a problem hiding this comment.
Adding inline here isn't that important, but it should be applied in node_realm.h, next to virtual.
And since this is a virtual method, I think you'll want to mark the PrincipalRealm and ShadowRealm classes as final as well, otherwise the compiler won't be able to know that there's no subclass which overrides the method (in other words, otherwise the compiler won't know that the method can actually be inlined)
There was a problem hiding this comment.
(and make it so Environment::principal_realm() returns a PrincipalRealm* pointer rather than a Realm* pointer – seems compilers can't always see through the cast that currently happens in that method)
3 participants
I tried to prevent the call from being repeated